cargo install multiple crates by durka · Pull Request #4216 · rust-lang/cargo

durka · 2017-06-23T23:36:00Z

rust-lang/rustup#986 for cargo install

Revives #2601 @pwoolcoc, replaces #3075 @esclear, closes #2585 @kindlychung @cyplo

Avoids the sticking point of the previous two PRs (multiple registry updates) by threading through a first-run boolean flag to decide whether select_pkg needs to call source.update().

There is still the issue that flags such as --git and --vers are "global" to the multiple packages you may be installing. The workaround is just to run cargo install separately. In the future we could add syntax like cargo install foo=1.0 bar=2.5 quux=git://github.com/durka/quux#dev-branch or something.

durka · 2017-06-24T04:19:16Z

Fixed tests.

durka · 2017-06-24T04:22:35Z

Reverting the bail! changes because I thought bail! == panic!... oops.

cyplo · 2017-06-24T07:10:13Z

Cool ! LGTM ! :)

alexcrichton

Can you be sure to also add tests for installing multiple crates where one is an error for whatever reason?

alexcrichton · 2017-07-07T15:09:38Z

tests/install.rs

+[INSTALLING] foo v0.0.1
+[COMPILING] foo v0.0.1
+[FINISHED] release [optimized] target(s) in [..]
+[INSTALLING] {home}[..]bin[..]foo[..]


Can this warning be printed out only once?

alexcrichton · 2017-07-07T15:10:17Z

src/cargo/ops/cargo_install.rs

+        }
+
+        writeln!(io::stderr(),
+                 "\n\nSUMMARY\n\nSuccessfully installed: {}\n\nErrors:\n\t{}",


Could this use the standard shell methods for reporting what happened here? Also can the whitespace be trimmed down as it seems like it's quite a bit?

alexcrichton · 2017-07-07T15:10:42Z

src/cargo/ops/cargo_install.rs

+            let map = map.clone();
+            match install_one(root, map, Some(krate), source_id, vers, opts, force) {
+                Ok(()) => success.push(krate),
+                Err(e) => errors.push(format!("{}: {}", krate, e))


We've got standard error handling mechanisms which print contextual information, and this loses the error backtrace. Can this just be propagated outwards?

Basically I didn't want errors to be lost in a sea of command-line output, so I thought it'd be better to collect them at the end.

durka · 2017-07-12T20:29:33Z

Addressed comments.

durka · 2017-07-24T23:03:36Z

I... have absolutely no idea why that test is failing. With my changes, cargo install seems to die after "Blocking waiting for lock on crate metadata", but it looks to me like it's trying the same actions in the same order.

durka · 2017-07-24T23:28:34Z

Should be fixed now.

alexcrichton · 2017-07-25T14:36:24Z

src/cargo/ops/cargo_install.rs

+            match install_one(root, map, Some(krate), source_id, vers, opts, force, first) {
+                Ok(()) => succeeded.push(krate),
+                Err(e) => {
+                    opts.config.shell().error(e)?;


I think this'll want to use the top-level error handling functions in Cargo in the root src/cargo/lib.rs, there's a lot of contextual information in this error that the raw Display impl doesn't print out.

Updated to use ::handle_error, I assume that's what you meant?

alexcrichton · 2017-07-25T14:37:20Z

src/cargo/ops/cargo_install.rs

+            summary.push(format!("Successfully installed {}!", succeeded.join(", ")));
+        }
+        if !failed.is_empty() {
+            summary.push(format!("Failed to install {} (see error(s) above).", failed.join(", ")));


Should this schedule an error to be returned at the top level, to ensure that cargo returns a nonzero exit status?

Now if there are failures it looks like this:

$ target/debug/cargo install asdfasdf fdasfdsa Updating registry `https://github.com/rust-lang/crates.io-index` error: could not find `asdfasdf` in `registry https://github.com/rust-lang/crates.io-index` error: could not find `fdasfdsa` in `registry https://github.com/rust-lang/crates.io-index` Summary: Failed to install asdfasdf, fdasfdsa (see error(s) above). error: some crates failed to install

and exits with 101.

durka · 2017-07-25T15:25:32Z

I wasn't sure about that. What if there are multiple, can I return a Vec of errors? It gets simpler if we go back to stop-at-first-error but personally I really prefer the persistence behavior.

…

On Jul 25, 2017 10:37, "Alex Crichton" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/cargo/ops/cargo_install.rs <#4216 (comment)>: > + match install_one(root, map, Some(krate), source_id, vers, opts, force, first) { + Ok(()) => succeeded.push(krate), + Err(e) => { + opts.config.shell().error(e)?; + failed.push(krate) + } + } + first = false; + } + + let mut summary = vec![]; + if !succeeded.is_empty() { + summary.push(format!("Successfully installed {}!", succeeded.join(", "))); + } + if !failed.is_empty() { + summary.push(format!("Failed to install {} (see error(s) above).", failed.join(", "))); Should this schedule an error to be returned at the top level, to ensure that cargo returns a nonzero exit status? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4216 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC3n-rpS_AvlohRBvStF0ee9gWD5XGtks5sRf2igaJpZM4OEJF5> .

durka · 2017-07-25T20:19:18Z

src/cargo/ops/cargo_install.rs

Starting to accumulate a slightly worrying number of boolean flags here. :/

alexcrichton · 2017-07-28T15:40:06Z

@bors: r+

Looks good to me, thanks!

bors · 2017-07-28T15:40:07Z

📌 Commit ce2d69d has been approved by alexcrichton

bors · 2017-07-28T15:40:15Z

⌛ Testing commit ce2d69d with merge 043099730925ff28d6923f6a5cfc9ee155a80582...

bors · 2017-07-28T16:04:39Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 043099730925ff28d6923f6a5cfc9ee155a80582 to master...

bors · 2017-07-28T16:04:40Z

👀 Test was successful, but fast-forwarding failed: 422 Required status check "continuous-integration/appveyor/pr" is expected.

alexcrichton · 2017-07-28T16:07:06Z

@bors: retry

@pwoolcoc

cargo install multiple crates rust-lang/rustup#986 for `cargo install` Revives #2601 @pwoolcoc, replaces #3075 @esclear, closes #2585 @kindlychung @cyplo Avoids the sticking point of the previous two PRs (multiple registry updates) by threading through a first-run boolean flag to decide whether `select_pkg` needs to call `source.update()`. There is still the issue that flags such as `--git` and `--vers` are "global" to the multiple packages you may be installing. The workaround is just to run `cargo install` separately. In the future we could add syntax like `cargo install foo=1.0 bar=2.5 quux=git://github.com/durka/quux#dev-branch` or something.

bors · 2017-07-28T16:07:14Z

⌛ Testing commit ce2d69d with merge 3751e68...

bors · 2017-07-28T16:37:35Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 3751e68 to master...

Changelog: Version 1.21.0 (2017-10-12) ========================== Language -------- - [You can now use static references for literals.][43838] Example: ```rust fn main() { let x: &'static u32 = &0; } ``` - [Relaxed path syntax. Optional `::` before `<` is now allowed in all contexts.][43540] Example: ```rust my_macro!(Vec<i32>::new); // Always worked my_macro!(Vec::<i32>::new); // Now works ``` Compiler -------- - [Upgraded jemalloc to 4.5.0][43911] - [Enabled unwinding panics on Redox][43917] - [Now runs LLVM in parallel during translation phase.][43506] This should reduce peak memory usage. Libraries --------- - [Generate builtin impls for `Clone` for all arrays and tuples that are `T: Clone`][43690] - [`Stdin`, `Stdout`, and `Stderr` now implement `AsRawFd`.][43459] - [`Rc` and `Arc` now implement `From<&[T]> where T: Clone`, `From<str>`, `From<String>`, `From<Box<T>> where T: ?Sized`, and `From<Vec<T>>`.][42565] Stabilized APIs --------------- [`std::mem::discriminant`] Cargo ----- - [You can now call `cargo install` with multiple package names][cargo/4216] - [Cargo commands inside a virtual workspace will now implicitly pass `--all`][cargo/4335] - [Added a `[patch]` section to `Cargo.toml` to handle prepublication dependencies][cargo/4123] [RFC 1969] - [`include` & `exclude` fields in `Cargo.toml` now accept gitignore like patterns][cargo/4270] - [Added the `--all-targets` option][cargo/4400] - [Using required dependencies as a feature is now deprecated and emits a warning][cargo/4364] Misc ---- - [Cargo docs are moving][43916] to [doc.rust-lang.org/cargo](https://doc.rust-lang.org/cargo) - [The rustdoc book is now available][43863] at [doc.rust-lang.org/rustdoc](https://doc.rust-lang.org/rustdoc) - [Added a preview of RLS has been made available through rustup][44204] Install with `rustup component add rls-preview` - [`std::os` documentation for Unix, Linux, and Windows now appears on doc.rust-lang.org][43348] Previously only showed `std::os::unix`. Compatibility Notes ------------------- - [Changes in method matching against higher-ranked types][43880] This may cause breakage in subtyping corner cases. [A more in-depth explanation is available.][info/43880] - [rustc's JSON error output's byte position start at top of file.][42973] Was previously relative to the rustc's internal `CodeMap` struct which required the unstable library `libsyntax` to correctly use. - [`unused_results` lint no longer ignores booleans][43728] [42565]: rust-lang/rust#42565 [42973]: rust-lang/rust#42973 [43348]: rust-lang/rust#43348 [43459]: rust-lang/rust#43459 [43506]: rust-lang/rust#43506 [43540]: rust-lang/rust#43540 [43690]: rust-lang/rust#43690 [43728]: rust-lang/rust#43728 [43838]: rust-lang/rust#43838 [43863]: rust-lang/rust#43863 [43880]: rust-lang/rust#43880 [43911]: rust-lang/rust#43911 [43916]: rust-lang/rust#43916 [43917]: rust-lang/rust#43917 [44204]: rust-lang/rust#44204 [cargo/4123]: rust-lang/cargo#4123 [cargo/4216]: rust-lang/cargo#4216 [cargo/4270]: rust-lang/cargo#4270 [cargo/4335]: rust-lang/cargo#4335 [cargo/4364]: rust-lang/cargo#4364 [cargo/4400]: rust-lang/cargo#4400 [RFC 1969]: rust-lang/rfcs#1969 [info/43880]: rust-lang/rust#44224 (comment) [`std::mem::discriminant`]: https://doc.rust-lang.org/std/mem/fn.discriminant.html

Paul Woolcock and others added 3 commits June 23, 2017 16:52

old commit adding some support for multi-install

66168ac

update test

40226f5

make SourceConfigMap cloneable

f2084b7

durka added 2 commits June 24, 2017 00:28

support installing multiple crates

497c297

don't print summary of single crate install

a65fad0

durka force-pushed the install-multi branch from 86dc62c to a65fad0 Compare June 24, 2017 04:28

alexcrichton reviewed Jul 7, 2017

View reviewed changes

durka added 3 commits July 12, 2017 14:57

only print PATH warning once

daf4eab

use shell helpers to condense output

aa201ab

add test for failing package

f78fc7c

don't continue if single install errors

1800c43

alexcrichton reviewed Jul 25, 2017

View reviewed changes

use top level err handling

1c7c88a

durka force-pushed the install-multi branch from 9bfcc55 to 628494d Compare July 25, 2017 20:18

durka commented Jul 25, 2017

View reviewed changes

src/cargo/ops/cargo_install.rs Outdated

Copy link

Contributor Author

durka Jul 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting to accumulate a slightly worrying number of boolean flags here. :/

nonzero exit code if some crates didn't install

ce2d69d

durka force-pushed the install-multi branch from 628494d to ce2d69d Compare July 25, 2017 21:18

bors merged commit ce2d69d into rust-lang:master Jul 28, 2017

ehuss added this to the 1.21.0 milestone Feb 6, 2022

weihanglo mentioned this pull request May 14, 2024

cargo install A B C only updates A if A, B, C are outdated #13911

Open

Conversation

durka commented Jun 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

durka commented Jun 24, 2017

Uh oh!

durka commented Jun 24, 2017

Uh oh!

cyplo commented Jun 24, 2017

Uh oh!

alexcrichton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durka commented Jul 12, 2017

Uh oh!

durka commented Jul 24, 2017

Uh oh!

durka commented Jul 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durka Jul 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durka commented Jul 25, 2017 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Jul 28, 2017

Uh oh!

bors commented Jul 28, 2017

Uh oh!

bors commented Jul 28, 2017

Uh oh!

bors commented Jul 28, 2017

Uh oh!

bors commented Jul 28, 2017

Uh oh!

alexcrichton commented Jul 28, 2017

Uh oh!

bors commented Jul 28, 2017

Uh oh!

bors commented Jul 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

durka commented Jun 23, 2017 •

edited

Loading

durka Jul 25, 2017 •

edited

Loading